Skip to content

Fix XHTTP+mux incompatibility and TUN-mode route-flip race - #1

Open
querxx wants to merge 1 commit into
faustyu1:mainfrom
querxx:fix/xhttp-mux-and-tun-race
Open

Fix XHTTP+mux incompatibility and TUN-mode route-flip race#1
querxx wants to merge 1 commit into
faustyu1:mainfrom
querxx:fix/xhttp-mux-and-tun-race

Conversation

@querxx

@querxx querxx commented Jul 1, 2026

Copy link
Copy Markdown

Summary

Two reliability bugs found while running VLESS+REALITY+XHTTP through Veil's TUN mode against a real server:

  1. muxSettings() only disables mux for xtls-rprx-vision flow, not for XHTTP. XHTTP has its own dedicated multiplexing (xmux inside xhttpSettings) - stacking the legacy Xray mux outbound on top of an XHTTP transport doesn't fail immediately, it works briefly then the connection goes dead with no recovery. Verified with a controlled A/B test against a real server, identical config except the mux block: 0/20 requests succeeded with mux enabled vs. 20/20 with it disabled (sustained over 100s). Fix: also return nil from muxSettings when cfg.network == .xhttp.

  2. TUN mode flips the default route before the tunnel is actually passing traffic. connect() only waits for the local SOCKS port to start listening (waitForPort, a bare TCP check, near-instant) before calling bringUpTransport(), which for .tun mode immediately re-points the machine's default route at the tunnel. A listening SOCKS port doesn't mean the REALITY/XHTTP handshake to the remote server has completed - that can take noticeably longer than a plain TCP connect, especially on a cold first connect. Lose that race and the whole machine loses internet until the tunnel catches up or the user disconnects and reconnects. HealthProbe.throughSocks() already exists (used by the 30s watchdog) and does a real end-to-end SOCKS CONNECT - this PR runs it after waitForPort() and before bringUpTransport(), with retries (~7.5s total) to give REALITY/XHTTP room to finish its handshake.

Both fixes verified together: sustained TUN-mode sessions (6+ minutes, checked every 15s) with zero drops, versus reliably dying within ~1-2 minutes before the fix.

Test plan

  • swift build -c release succeeds
  • Controlled A/B test isolating mux as the sole variable against a real VLESS+REALITY+XHTTP server (0/20 vs 20/20)
  • Sustained TUN-mode session over 6+ minutes with periodic system-wide connectivity checks, zero failures
  • Verified against both the official upstream Xray-core release binary and the exact binary bundled in the app
  • Would appreciate a second pair of eyes on the retry/backoff constants in the health-probe loop (15 attempts * 0.5s = ~7.5s) - happy to tune if you have a sense of typical REALITY/XHTTP handshake latency across more servers

1. mux was incompatible with XHTTP transport, not just Vision flow

muxSettings() only skipped Xray's legacy `mux` when the flow was
xtls-rprx-vision. It didn't account for XHTTP, which has its own
dedicated multiplexing (the `xmux` block inside xhttpSettings).
Layering classic `mux` on top of an XHTTP outbound doesn't fail fast -
it works briefly and then the connection goes dead with no recovery.

Verified with a controlled A/B test against a real VLESS+REALITY+XHTTP
server, identical config except for the mux block:
  - mux enabled:  0/20 requests succeeded
  - mux disabled: 20/20 requests succeeded (sustained over 100s)

Fix: also skip mux whenever cfg.network == .xhttp.

2. TUN mode flipped the system default route before the tunnel was
   actually passing traffic

connect() polled the local SOCKS port with waitForPort() (a bare TCP
connect check, near-instant) and then immediately called
bringUpTransport(), which for .tun mode flips the machine's default
route to the tunnel via tun-up.sh. A listening SOCKS port doesn't mean
the REALITY/XHTTP handshake to the remote server has completed yet -
that reliably takes longer than a plain TCP connect, especially on a
cold first connect. If the race is lost, the default route points at
a tunnel that isn't passing traffic yet, and the whole machine loses
internet until the tunnel catches up or the user disconnects.

HealthProbe.throughSocks() (already used by the 30s watchdog) does a
real end-to-end SOCKS CONNECT proving the full path works. Fix: run it
after waitForPort() and before bringUpTransport(), with retries (up to
~7.5s) to give REALITY/XHTTP room to finish its handshake, instead of
gating solely on "is the local port listening."

Both fixes verified together end-to-end: sustained TUN-mode sessions
(6+ minutes, checked every 15s) with zero drops, vs. reliably dying
within ~1-2 minutes before the fix.

@faustyu1 faustyu1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed both commits against a local checkout of the branch, reading ConnectionManager, HealthProbe, TunManager and Resources/tun-up.sh together rather than just the diff.

Summary: fix #1 (mux + XHTTP) looks correct and I'd take it as-is. Fix #2 addresses a real race on first connect, but as written it breaks server switching in TUN mode and makes the "whole machine loses internet" outcome more likely, not less. Details below.


🔴 Blocker: the probe deadlocks when switching servers in TUN mode

Sources/XrayClient/Core/ConnectionManager.swift:166-175 runs the probe before bringUpTransport() — but bringUpTransport()TunManager.up()tun-up.sh is exactly what pins the server IP to the physical gateway (Resources/tun-up.sh:56-62, the fast re-pin path).

Scenario: TUN is up on server A, user picks server B.

  1. connect() takes the keepTransport == true path (ConnectionManager.swift:85) — TUN and the default route stay in place, and only A's IP is pinned.
  2. xray restarts with B's config. waitForPort passes almost instantly.
  3. The probe goes into SOCKS → xray dials B's IP. B's IP is not pinned yet, so those packets follow 0.0.0.0/1utun123 → tun2socks → the same SOCKS port → xray → loop.
  4. All 15 attempts fail, then xray.stop() + fail("tunnel did not pass traffic in time").

End state: TUN up, default route into the tunnel, DNS rewritten to 1.1.1.1, and xray dead — no internet at all until the user manually hits Disconnect. This isn't a race, it's deterministic on every TUN server switch unless A and B happen to resolve to the same IP.

Fix direction — pinning the server IP has to be separated from flipping the default route:

  • quick: skip the probe entirely when chosenMode == .tun && keepTransport (the route is already up and the probe can't be valid there anyway);
  • proper: add a "pin only" mode to tun-up.sh (step 3 without steps 4-5), call it before the probe, and only then bringUpTransport().

🟠 The failure path leaves the system offline with no auto-recovery

ConnectionManager.swift:179-182 calls xray.stop() + fail(), and fail() (line 283) does not call teardownTransport(). Before this PR that branch was nearly unreachable (a local listening socket almost always comes up); now it fires on any transient unreachability of the remote server.

This hurts most on a watchdog-driven reconnect (reconnect()connect(), line 295): a brief network blip → probe fails → xray killed → state == .failed → the watchdog loop (line 320) sees state != .connected, treats itself as "busy", resets the counter, and never reconnects again. Previously xray stayed alive and the watchdog kept retrying. So a transient failure becomes permanent, with the routes still pointing into a dead tunnel.

Minimum: call teardownTransport() before fail() when activeMode == .tun. Better: when keepTransport is true, don't kill xray at all — let the watchdog own recovery.

🟡 The gate also applies to systemProxy, where there was no risk

The probe sits before the switch mode and is equally fatal in both modes. In .systemProxy this turns "slow but working server" from connected, watchdog will handle it into connection failed — a regression with no upside, since there's no default-route race there. I'd scope the probe to .tun only.

🟡 The probe target 1.1.1.1:80 interacts with user routing rules

HealthProbe.throughSocks defaults to 1.1.1.1:80 (HealthProbe.swift:13), while the config is built with the user's rules (XrayConfigBuilder.routing). A direct rule covering that IP (or geoip:cloudflare) makes the probe succeed without traversing the tunnel — which defeats the guarantee this PR is buying. A block rule makes connecting impossible altogether. Tolerable for the watchdog, not for a connect gate. Worth probing a target that is guaranteed to hit the proxy outbound.

On the retry constants (your open question)

15 × 0.5s = ~7.5s in the description only counts the sleeps. The probe itself has a 2s timeout (ConnectionManager.swift:170), and against a server that accepts the connection but never answers, each iteration burns its full 2s: worst case is roughly 37.5s stuck in "Connecting…". I'd switch from an attempt count to a wall-clock deadline:

let deadline = Date().addingTimeInterval(10)
while Date() < deadline { ... }

That way behaviour no longer depends on whether the server refuses instantly or just goes silent.

Nits

  • XrayConfigBuilder.swift:81-83: the comment says "the xmux block inside xhttpSettings", but the builder never emits xmux (see the .xhttp branch of streamSettings) — that's Xray's own default. Worth rewording so nobody goes looking for a block that isn't there.
  • LinkParser.swift:346: type=splithttp (the older XHTTP alias, still emitted by panels like 3x-ui) falls through to ?? .tcp, so the new cfg.network == .xhttp check never fires for those links. Pre-existing and broader than this PR, but it directly limits how much fix #1 buys — adding the alias would be a good companion change.
  • No tests. XrayConfigBuilder.build is reachable from the test target (@testable import XrayClient), so "outbound has no mux key when network is .xhttp" is a five-line test that locks the regression out for good.
  • ConnectionManager.swift:150-152: argument alignment for port:/timeout: drifted after the variable rename.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants